Merged
Conversation
Replaces #4238 To avoid SDK users accidentally using the DSN used internally to maintain the Sentry SDK, the DSN has been removed from all of the sample projects. SDK maintainers can either set this as an environment variable or, as with other SDK users, set it manually in the `samples/SamplesShared.cs` file (although changes to that file should never be checked in). #skip-changelog
bruno-garcia
approved these changes
Jun 9, 2025
| #if !SENTRY_DSN_DEFINED_IN_ENV | ||
| // You must specify a DSN. On mobile platforms, this should be done in code here. | ||
| // See https://docs.sentry.io/product/sentry-basics/dsn-explainer/ | ||
| options.Dsn = SamplesShared.Dsn; |
Member
There was a problem hiding this comment.
This will just be overwritten by the one below. I don't follow, why do we do this?
Collaborator
Author
There was a problem hiding this comment.
Not overwritten no... this code gets compiled only #if !SENTRY_DSN_DEFINED_IN_ENV - so when the SENTRY_DSN environment variable isn't set. This will typically be the case for SDK users who have forked the repo.
If the environment variable is set then we write it into EnvironmentVariables.Dsn where it's available to mobile samples (like this one)... which is where we read it from below. This is probably what will happen when SDK maintainers are running the samples.
Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #4238
Summary
To avoid SDK users accidentally using the DSN used internally to maintain the Sentry SDK, the DSN has been removed from all of the sample projects. SDK maintainers can either set this as an environment variable or, as with other SDK users, set it manually in the
samples/SamplesShared.csfile (although changes to that file should never be checked in).Problem
In the past, I would check out a branch, manually copy/paste our DSN into one of the sample projects, work on the code and then try to remember not to check in that file/change... unless the changes actually concerned the sample projects themselves, in which case I'd have to remove the DSN before making the commit. For a single commit, that's not too much effort. However when you're switching branches 10 times a day and doing that every day of every week, it starts to get pretty repetitive and error prone. Over time then, we just ended up leaving the DSNs in the sample projects... and a week or so ago one of our SDK users must have accedentally left our sample DSN in one of their apps which is now in staging or production or something.
Approach
The idea is that two main groups of people might be forking this repo:
SDK Users
For SDK users, we want to make it as easy as possible for them to run any of our sample apps. One of the most common scenarios would be for someone unfamiliar with Sentry to try to run one of the samples without making any changes... which won't work because, at the very least, they need to specify a DSN.
We could wait until the application runs and then log an error... and that's what would happen if you created a new .NET application, added the Sentry package and tried to initialise Sentry without any DSN.
In the Samples for this repo though we can catch the problem at compile time, rather than waiting until runtime... for this, there's a new /samples/SamplesShared.cs file that, by default, results in a compilation error... letting any potential SDK users or new contributors know that they need to provide a DSN before running the sample.
Maintainers and contributors
For people who are contributing to the SDK regularly, I figured we could just set the DSN in an environment variable and have all of the samples automatically read it from that... if a DSN isn't specified explicitly via the options (or if it is but it's an empty DSN) this would be the default behaviour anyway.
About the only slight glitch with that approach is that Android, iOS and MacCatalyst apps won't pick up environment variables... and AssemblyAttributes also don't work for these applications since
Assembly.GetEntryAssembly()returns null for these apps.As a workaround, this PR adds a
GenerateSharedDsnConstantbuild target that writes theSENTRY_DSNenvironment variable out as a constant to a generated file that gets included in all of the sample apps... and this DSN can then be referenced directly from any of the mobile samples.Notes re tests
Some of the tests never actually worked if the
SENTRY_DSNenvironment variable was set. Those tests are now skipped when this variable is set (which should/will never be the case on CI).Also the code that would result in an error compiling the samples when the DSN is neither specified as an environment variable nor in code is conditionally compiled only when we are not building the solution on CI... on CI the samples will compile fine with no DSN configured. They would bomb if you actually tried to run them, but that should never be an issue.
#skip-changelog